feat: Support set custom TF/OpenTofu binary. | If you use a custom Docker image build, please note that `TERRAFORM_VERSION` now must be provided #670

den-is · 2024-05-04T15:05:23Z

Put an x into the box if that apply:

This PR introduces breaking change.
This PR fixes a bug.
This PR adds new functionality.
This PR enhances existing functionality.

Description of your changes

Allow setting custom TF binary.

provide per hook setting --hook-config=--tf-path=<path_to_binary_or_binary_name>
export ENV variable PCT_TFPATH=<path_to_binary_or_binary_name>
export ENV variable TERRAGRUNT_TFPATH=<path_to_binary_or_binary_name>
if none of above are set defaults to terraform binary found in the user's $PATH
if none of above are set/available defaults to tofu binary found in the user's $PATH
in the end, if none of above is set/available - exit 1 with error message

Side feature. Fixes bad README.md

I'm open to any suggestion and contributions.

Fixes #570

How can we test changes

Create dummy repository with a dummy terraform module test

mkdir test
cd test
git init

Add files to test/:
.pre-commit-config.yaml

repos:
- repo: local
  hooks:
  - id: terraform_fmt
    name: terraform_fmt
    entry: /path/to/pre-commit-terraform/hooks/terraform_fmt.sh
    language: script
    files: \.tf$
    # Uncomment these lines. Or set global ENV variables PCT_TFPATH or TERRAGRUNT_TFPATH
    #args:
    #- --hook-config=--tf-path=tofu

main.tf

# dummy terraform file with ugly formatting
resource "null_resource" "print_time" {
triggers = {
current_time = timestamp()
}

provisioner "local-exec" {
command = "echo Current time is ${triggers.current_time}"
}
}

Run in the test module directory:

pre-commit install
pre-commit run --verbose --files main.tf

Co-authored-by: George L. Yermulnik <[email protected]>

MaxymVlasov · 2024-05-21T16:38:07Z

Ah, it discared full readme, not just this commit, meh, 1 min

This reverts commit 3660e75.

This reverts commit 258234f.

hooks/_common.sh

den-is · 2024-05-21T16:45:39Z

Pushed final README fixes. Missed the broken README part the last time when I was reverting back to */

MaxymVlasov · 2024-05-21T17:26:59Z

hooks/_common.sh

+  # Maybe there is a similar setting for Terragrunt already
+  elif [[ $TERRAGRUNT_TFPATH ]]; then
+    echo "${TERRAGRUNT_TFPATH}"
+    return


I don't get it

According to https://terragrunt.gruntwork.io/docs/reference/cli-options/#terragrunt-tfpath it will be automatically specified for terragrunt => for terragrunt hooks too.

Why do we need to set Terragrunt path for vanilla TF hooks?

Imagine you are working in some environment that already uses TERRAGRUNT_TFPATH env variable (global env, using direnv, or any other method)
So if a variable already exists and serves exactly the same purposes, why not to check it and reuse it, why to have other args/vars/etc?

Not sure I'm following the gist of the question, though probably (I'm purely guesstimating) this way it would be easier for people using both TF and TG hooks to specify custom binary only once using TG's method but covering also TF? 🤔 And from this point of view it makes sense to me.

@den-is What's your idea?

Not sure I'm following the gist of the question, though probably (I'm purely guesstimating) this way it would be easier for people using both TF and TG hooks to specify custom binary only once using TG's method but covering also TF? 🤔 And from this point of view it makes sense to me.

@yermulnik Erm... this was already replied to by me yesterday.

Folks that use terragrunt should use terragrunt hooks, not terraform hooks - terraform will just not work in most cases. It's hard to imagine that someone uses both terragrunt and vanilla terraform, but if so - why do we think that folks would be happy with interference between "prj1 with TG and OpenTofu" and "prj2 with vanilla TF"?

And another question: do we need to specify $tf_path for terragrunt hooks?

Like terragrunt validate --terragrunt-tfpath "$tf_path" "${args[@]}"

pre-commit-terraform/hooks/terragrunt_validate.sh

Line 46 in 1ac4f2d

terragrunt validate "${args[@]}"

Erm... this was already replied to by me yesterday.

@den-is My bad. I missed that comment (found it now). Thanks.

It's hard to imagine that someone uses both terragrunt and vanilla terraform

I don't in particular, but a fellow squad in my project — does. And I'd imagine there are people in the wild who do the same and hence they may like using TERRAGRUNT_TFPATH to cover both use cases if they use pre-commit-terraform.

And another question: do we need to specify $tf_path for terragrunt hooks?

That's a good question. We probably should. But only when it is provided via TG hook config. In the rest of use cases TG should rely upon its own logic, including TERRAGRUNT_TFPATH and others described at https://terragrunt.gruntwork.io/docs/reference/cli-options/#terragrunt-tfpath

Had a conversation with @den-is
TL;DR: we assume that it will no break anything in 99% of cases. And if such a thing happened - I'll mention Den is such an issue :D

Works for me 👍🏻

I'll mention Den is such an issue :D

I feel like is should be in, but with is the sentence sounds fabulous 🤣

hooks/_common.sh

README.md

hooks/_common.sh

yermulnik · 2024-05-22T11:10:14Z

hooks/_common.sh

+  # Maybe there is a similar setting for Terragrunt already
+  elif [[ $TERRAGRUNT_TFPATH ]]; then
+    echo "${TERRAGRUNT_TFPATH}"
+    return


Not sure I'm following the gist of the question, though probably (I'm purely guesstimating) this way it would be easier for people using both TF and TG hooks to specify custom binary only once using TG's method but covering also TF? 🤔 And from this point of view it makes sense to me.

@den-is What's your idea?

hooks/_common.sh

tools/install/opentofu.sh

Co-authored-by: George L. Yermulnik <[email protected]>

yermulnik

LGTM apart from #670 (comment) that I didn't get why TF switched from latest to exact version.

README.md

den-is · 2024-05-09T07:40:03Z

README.md

@@ -103,7 +115,7 @@ All available tags [here](https://github.com/antonbabenko/pre-commit-terraform/p
 **Build from scratch**:

 > [!IMPORTANT]
-> To build image you need to have [`docker buildx`](https://docs.docker.com/build/install-buildx/) enabled as default builder.  
+> To build image you need to have [`docker buildx`](https://docs.docker.com/build/install-buildx/) enabled as default builder.


This was fixed by VSCode automatically.
Will check for workaround

hooks/_common.sh

README.md

den-is · 2024-05-21T17:31:11Z

hooks/_common.sh

+  # Maybe there is a similar setting for Terragrunt already
+  elif [[ $TERRAGRUNT_TFPATH ]]; then
+    echo "${TERRAGRUNT_TFPATH}"
+    return


Imagine you are working in some environment that already uses TERRAGRUNT_TFPATH env variable (global env, using direnv, or any other method)
So if a variable already exists and serves exactly the same purposes, why not to check it and reuse it, why to have other args/vars/etc?

README.md

den-is · 2024-05-22T12:00:28Z

hooks/_common.sh

+  # Maybe there is a similar setting for Terragrunt already
+  elif [[ $TERRAGRUNT_TFPATH ]]; then
+    echo "${TERRAGRUNT_TFPATH}"
+    return


Not sure I'm following the gist of the question, though probably (I'm purely guesstimating) this way it would be easier for people using both TF and TG hooks to specify custom binary only once using TG's method but covering also TF? 🤔 And from this point of view it makes sense to me.

@yermulnik Erm... this was already replied to by me yesterday.

yermulnik · 2024-05-22T16:17:05Z

This was fixed by VSCode automatically.
Will check for workaround

Add the below to config of VSCode of yours:

    "[markdown]": {
        "files.trimTrailingWhitespace": false
    }

Other common useful settings VSCode usually is missing from what I've found from experience of others using it:

    "files.trimTrailingWhitespace": true,
    "files.insertFinalNewline": true,
    "files.trimFinalNewlines": true,

den-is · 2024-05-22T16:48:48Z

This was fixed by VSCode automatically.
Will check for workaround

Add the below to config of VSCode of yours:
    "[markdown]": {
        "files.trimTrailingWhitespace": false
    }
Other common useful settings VSCode usually is missing from what I've found from experience of others using it:
    "files.trimTrailingWhitespace": true,
    "files.insertFinalNewline": true,
    "files.trimFinalNewlines": true,

lol dude... i always have this setting in all my editors... for years... you basically copy pasted my snippets which I add to different projects were noobs can't solve their whitespace issues.

Only thing I was not able to find correct settings for VSCode to fix * replacement with -. (Well I did not spend hours to find it in VSCode... just used vim to revert to your standard)

MaxymVlasov · 2024-05-22T17:01:17Z

Space trimming and so on should be done by .editorconfig in the first place, as is supported by everything, including vim.

TOC is not supported by built-in MD engine, for this I use https://marketplace.visualstudio.com/items?itemName=yzhang.markdown-all-in-one

With

    "markdownlint.config": {
        "MD029": false,
        "MD012": {
            "maximum": 5
        }
    },
    "markdown.extension.toc.unorderedList.marker": "*",
    "markdown.extension.toc.levels": "2..6",

I'll check vanilla VS Code settings and then will send PR which will add needed configs and/or rewrite MD file

den-is · 2024-05-22T17:08:01Z

In case if person uses VSCode, most of .editorconfig settings are supported natively. Otherwise you have to install another extension.

But yea, addined .editorconfig to you project makes it clear what formatting you require and ask to match. (from those who can't configure their IDEs with universal/common/common-sense/not-personal settings)

yermulnik · 2024-05-22T17:24:45Z

Only thing I was not able to find correct settings for VSCode to fix * replacement with -. (Well I did not spend hours to find it in VSCode...

It's probably not the VSCode per se, but an extension. Like some of these: https://github.com/yzhang-gh/vscode-markdown or https://github.com/prettier/prettier-vscode

BTW FWIW: yzhang-gh/vscode-markdown#145

just used vim to revert to your standard)

It's not ours. It's Markdown language specification. Unless you prove otherwise.

PS: I looked at markdownlint rules and they seem to prefer * over - for unordered lists in documentation — https://github.com/DavidAnson/markdownlint/blob/main/doc/Rules.md 🤔

# [1.90.0](v1.89.1...v1.90.0) (2024-05-23) ### Features * Support set custom TF/OpenTofu binary. | If you use a custom Docker image build, please note that `TERRAFORM_VERSION` now must be provided ([#670](#670)) ([c7011c0](c7011c0))

antonbabenko · 2024-05-23T03:08:06Z

This PR is included in version 1.90.0 🎉

Allowing setting custom TF binary

c85dfd5

den-is requested review from MaxymVlasov and yermulnik as code owners May 4, 2024 15:05

den-is changed the title ~~Allowing setting custom TF binary~~ feat: Allow setting custom TF binary May 4, 2024

README update

30674bc

This comment was marked as resolved.

Sign in to view

den-is and others added 14 commits May 9, 2024 11:28

Update README.md

af193f1

Co-authored-by: George L. Yermulnik <[email protected]>

Update README.md

43edcea

Co-authored-by: George L. Yermulnik <[email protected]>

Update hooks/_common.sh

78f7f10

Co-authored-by: George L. Yermulnik <[email protected]>

Update hooks/_common.sh

45706a8

Co-authored-by: George L. Yermulnik <[email protected]>

Update hooks/_common.sh

4018878

Co-authored-by: George L. Yermulnik <[email protected]>

Update hooks/_common.sh

c25e0fb

Co-authored-by: George L. Yermulnik <[email protected]>

Update hooks/_common.sh

7c448a6

Co-authored-by: George L. Yermulnik <[email protected]>

Update hooks/_common.sh

b1dd3bb

Co-authored-by: George L. Yermulnik <[email protected]>

refactoring

2e353ba

update terraform_providers_lock.sh

8ca6f2d

README update

d3c85b4

add globals doc for common::get_tf_path

c73c2e8

centralize TF_PATH initialization

5301302

fix expected hook-config arg

53146fe

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

fix command checks

a6fedca

This comment was marked as resolved.

Sign in to view

den-is added 3 commits May 11, 2024 13:26

README update

2f6f7b0

README update

200b96b

README update

5091208

MaxymVlasov added 2 commits May 21, 2024 19:39

Revert "Discard changes to README.md"

6670e51

This reverts commit 3660e75.

Revert "README update"

825b7f0

This reverts commit 258234f.

MaxymVlasov reviewed May 21, 2024

View reviewed changes

hooks/_common.sh Outdated Show resolved Hide resolved

den-is force-pushed the allow-setting-tf-path branch from 825b7f0 to 35e7f1b Compare May 21, 2024 16:43

Switch from global env to local in-function constant

bc2766c

MaxymVlasov force-pushed the allow-setting-tf-path branch from 35e7f1b to bc2766c Compare May 21, 2024 17:23

MaxymVlasov reviewed May 21, 2024

View reviewed changes

MaxymVlasov added 2 commits May 21, 2024 20:30

fixup. Useless suppresions

168dd6a

Add tofu to Docker, make terraform and tofu non-required deps

80d239d

MaxymVlasov changed the title ~~feat: Allow setting custom TF binary~~ feat: Support set custom TF/OpenTofu binary. | If you use a custom Docker image build, please note that TERRAFORM_VERSION now must be provided May 21, 2024

Add tofu to ISSUE template

c7ad7d1

MaxymVlasov reviewed May 21, 2024

View reviewed changes

hooks/_common.sh Show resolved Hide resolved

yermulnik reviewed May 22, 2024

View reviewed changes

MaxymVlasov and others added 2 commits May 22, 2024 15:27

Apply suggestions from code review

e3c66bd

Co-authored-by: George L. Yermulnik <[email protected]>

Extract only need binary file

1cb551e

yermulnik approved these changes May 22, 2024

View reviewed changes

den-is commented May 22, 2024

View reviewed changes

MaxymVlasov approved these changes May 22, 2024

View reviewed changes

antonbabenko approved these changes May 23, 2024

View reviewed changes

MaxymVlasov merged commit c7011c0 into antonbabenko:master May 23, 2024
6 checks passed

MaxymVlasov mentioned this pull request Oct 10, 2024

fix: Support custom TF paths which contains spaces #714

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support set custom TF/OpenTofu binary. | If you use a custom Docker image build, please note that `TERRAFORM_VERSION` now must be provided #670

feat: Support set custom TF/OpenTofu binary. | If you use a custom Docker image build, please note that `TERRAFORM_VERSION` now must be provided #670

den-is commented May 4, 2024 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

MaxymVlasov commented May 21, 2024

den-is commented May 21, 2024

MaxymVlasov May 21, 2024

den-is May 21, 2024

yermulnik May 22, 2024

den-is May 22, 2024

MaxymVlasov May 22, 2024

yermulnik May 22, 2024

yermulnik May 22, 2024

MaxymVlasov May 22, 2024

yermulnik May 22, 2024

yermulnik May 22, 2024

yermulnik left a comment

den-is May 9, 2024

den-is May 21, 2024

den-is May 22, 2024

yermulnik commented May 22, 2024 •

edited

Loading

den-is commented May 22, 2024

MaxymVlasov commented May 22, 2024 •

edited

Loading

den-is commented May 22, 2024 •

edited

Loading

yermulnik commented May 22, 2024

antonbabenko commented May 23, 2024

feat: Support set custom TF/OpenTofu binary. | If you use a custom Docker image build, please note that TERRAFORM_VERSION now must be provided #670

feat: Support set custom TF/OpenTofu binary. | If you use a custom Docker image build, please note that TERRAFORM_VERSION now must be provided #670

Conversation

den-is commented May 4, 2024 • edited Loading

Description of your changes

How can we test changes

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

MaxymVlasov commented May 21, 2024

den-is commented May 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yermulnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yermulnik commented May 22, 2024 • edited Loading

den-is commented May 22, 2024

MaxymVlasov commented May 22, 2024 • edited Loading

den-is commented May 22, 2024 • edited Loading

yermulnik commented May 22, 2024

antonbabenko commented May 23, 2024

feat: Support set custom TF/OpenTofu binary. | If you use a custom Docker image build, please note that `TERRAFORM_VERSION` now must be provided #670

feat: Support set custom TF/OpenTofu binary. | If you use a custom Docker image build, please note that `TERRAFORM_VERSION` now must be provided #670

den-is commented May 4, 2024 •

edited

Loading

yermulnik commented May 22, 2024 •

edited

Loading

MaxymVlasov commented May 22, 2024 •

edited

Loading

den-is commented May 22, 2024 •

edited

Loading